Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add environment variables to the system process metricset #3337

Merged
merged 2 commits into from
Jan 12, 2017
Merged

Add environment variables to the system process metricset #3337

merged 2 commits into from
Jan 12, 2017

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Jan 11, 2017 •
edited
Loading

This PR adds the environment variables that were used to start the process to the data reported in the system process metricset. The data is added as a dictionary under the system.process.env key. Environment variables must be whitelisted using an array of regular expressions specified using process.env.whitelist: [] in the module config.

This feature implemented for FreeBSD, Linux, and OS X.

Sample config:

metricbeat.modules:
- module: system
  metricsets: [process]
  process.env.whitelist: ['USER', 'PATH']

@andrewkroh andrewkroh added enhancement in progress Pull request is currently in progress. Metricbeat Metricbeat labels Jan 11, 2017
@andrewkroh
Copy link
Member Author

What kind of configuration options if any (like filtering, whitelist, blacklist, enabled) should we provide with this feature?

@ruflin
Copy link
Contributor

ruflin commented Jan 11, 2017

I would only add process.env.enabled: true as a config option. Filtering can be done with the processors if needed. I think with the processors we should also be able to only have the variables for certain processes and remove it in all other cases etc.

I'm unsure if we should enabled it by default. On the one hand the info can be quite useful, but it can also contain quite a list of defaults which are the same for all processes.

@tsg
Copy link
Contributor

tsg commented Jan 11, 2017

@joegallo @alexbrasetvik FYI, we're going this direction for Metricbeat.

@tsg
Copy link
Contributor

tsg commented Jan 11, 2017

I'd say we should have a whitelist and the feature should be disabled by default due to privacy concerns. Secrets are often stored in env, so we have to make it hard to leak them accidentally.

@tsg
Copy link
Contributor

tsg commented Jan 11, 2017

To be clear, I mean a whitelist on the variable names, so something like process.env.variables: ["PATH"].

@andrewkroh andrewkroh force-pushed the feature/process-env-vars branch from 836d24d to bd054f9 Compare January 11, 2017 22:28
@andrewkroh
Copy link
Member Author

andrewkroh commented Jan 11, 2017 •
edited
Loading

I added process.env.whitelist: [] to control what environment variables are added to the system process events. It accepts a list of regular expressions. When empty or unset, no environment variables are added to the process event.

This should be updated to the the ExactMatcher from #2433 when it merges.

This PR depends on elastic/gosigar#62 and needs to be updated after that merges to put the commit hash into the glide.yaml. (This PR has been updated.)

ruflin
ruflin approved these changes Jan 12, 2017
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @andrewkroh I merged elastic/gosigar#62.

@andrewkroh
Update gosigar version to b49e01eb1e5c68c469392a63feaccae9352ceb12
5f5ec13 
This gets the ProcArgs implementation.
@andrewkroh
Add environment variables to the system process metricset
6e23846 
This PR adds the environment variables that were used to start the process to the data reported in the system process metricset. The data is added as a dictionary under the `system.process.env` key. Environment variables must be whitelisted using an array of regular expressions specified using `process.env.whitelist: []` in the module config.

This feature implemented for FreeBSD, Linux, and OS X.
@andrewkroh andrewkroh force-pushed the feature/process-env-vars branch from bd054f9 to 6e23846 Compare January 12, 2017 15:00
@andrewkroh andrewkroh added review and removed in progress Pull request is currently in progress. labels Jan 12, 2017
@andrewkroh
Copy link
Member Author

The PR is now updated with the final gosigar commit hash.

@ruflin ruflin merged commit 82b28c6 into elastic:master Jan 12, 2017
andrewkroh added a commit to andrewkroh/beats that referenced this pull request Feb 28, 2017
@andrewkroh
Document process.env.whitelist config option
00b5a5c 
Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for elastic#3337.
@andrewkroh andrewkroh mentioned this pull request Feb 28, 2017
Merged
ruflin pushed a commit that referenced this pull request Mar 3, 2017
@andrewkroh @ruflin
Document process.env.whitelist config option (#3694)
f9ac9f2 
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for #3337.

* Add reason behind default behavior for process env vars
monicasarbu pushed a commit to monicasarbu/beats that referenced this pull request Mar 4, 2017
@andrewkroh @monicasarbu
Document process.env.whitelist config option (elastic#3694)
2226d63 
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for elastic#3337.

* Add reason behind default behavior for process env vars

(cherry picked from commit f9ac9f2)
@monicasarbu monicasarbu mentioned this pull request Mar 4, 2017
Merged
ruflin pushed a commit that referenced this pull request Mar 6, 2017
@monicasarbu @ruflin
Document process.env.whitelist config option (#3694) (#3722)
3bd3287 
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for #3337.

* Add reason behind default behavior for process env vars

(cherry picked from commit f9ac9f2)
@monicasarbu monicasarbu deleted the feature/process-env-vars branch March 14, 2017 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

Assignees
No one assigned
Labels
enhancement Metricbeat Metricbeat review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrewkroh @ruflin @tsg